Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Alert not rendering in dark theme if XamlRoot or window is dark #8777

Merged
merged 2 commits into from
Oct 8, 2021

Conversation

lyahdav
Copy link
Collaborator

@lyahdav lyahdav commented Oct 4, 2021

This works around XAML bug microsoft/microsoft-ui-xaml#2331. It happens if a Window or XamlRoot's Content element has RequestedTheme set to Dark and then you open a ContentDialog in that window. To workaround we explicitly set the RequestedTheme on the Popup.

To test I have my OS color set to light, run playground app, set the theme to dark in the app, then open an Alert.

Before playground-win32:
new-win32-before

After playground-win32:
new-win32-after

Before playground (UWP):
new-uwp-before

After playground (UWP):
new-uwp-after

Microsoft Reviewers: Open in CodeFlow

@lyahdav lyahdav requested a review from a team as a code owner October 4, 2021 20:57
@@ -71,6 +74,22 @@ void Alert::ProcessPendingAlertRequests() noexcept {
}
}

// Workaround XAML bug with ContentDialog and dark theme:
// https://github.com/microsoft/microsoft-ui-xaml/issues/2331
dialog.Opened([useXamlRootForThemeBugWorkaround](winrt::IInspectable const &sender, auto &&) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this only repros in islands, right? if so, we should only do this dance if the app is an islands app, there are some helpers we use elsewhere for detecting this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not only Islands. You can see the bug (and fix) in UWP in screenshots above.

@NickGerleman
Copy link
Collaborator

@msftbot merge in 48 hours

@ghost ghost added the AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) label Oct 6, 2021
@ghost
Copy link

ghost commented Oct 6, 2021

Hello @NickGerleman!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Fri, 08 Oct 2021 23:04:00 GMT, which is in 2 days

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@NickGerleman NickGerleman merged commit 5310dcb into microsoft:main Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants